BluetoothSerial : Re-set _isRemoteAddressSet to false if connect() fails #6728
Conversation
…ils. The internal _isRemoteAddressSet variable is set to true when calling connect() functions. If connecting fails _isRemoteAddressSet needs to be re-set to false, otherwise other functions, such as discover() will fail without clear error messages.
|
|
There was a problem hiding this comment.
Good catch. After studying BluetoothSerial.cpp, I can see the reason for _isRemoteAddressSet. This is a necessary bugfix. Checked program flow of new logic. Appears functionally equivalent. LGTM.
I will note that if _peer_bd_addr was a uint64_t instead of byte array, it could be checked for non-zero and replace the flag _isRemoteAddressSet. That's how I'm handling MAC addresses in my project, which utilizes the new library "MacAddress.h". Of course then you'd need to convert to the byte array when passing _peer_bd_addr to the native IDF functions.
|
Honestly, the entire BluetoothSerial.cpp could "use some improvement" regarding indentation and logging/debugging, and possibly also some parts of the functionality. But I hesitate to submit a big whitespace patch, as such a thing would make it look like i wrote the entire thing if such a patch would be accepted. Also, do I need to do anthing more to get attention to this and get it pulled? |
This will happen soon. We are just focused on other parts of the repository at the moment. |
Description of Change
The BluetoothSerial library uses an internal _isRemoteAddressSet variable to track when a remote address has
been given. The connect() calls set this under certain circumstances, but fails to set it back to false if connection
fails. This causes subsequent calls to other functions such as discover() to fail without clear indications why.
Tests scenarios
I have tested this change on an Sparkfun ESP32 Thing Plus and connect() and subsequent discover() calls work as expected.